Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Plugin validator should support nunjucks functions #2375

Merged

Conversation

BenSurgisonGDS
Copy link
Contributor

The plugins validator should allow nunjucks functions to be included by validating the prototype config key nunjucksFunctions

@BenSurgisonGDS BenSurgisonGDS force-pushed the plugin-validator-should-support-nunjucks-functions branch from ee00aee to 20728dd Compare November 17, 2023 09:24
@BenSurgisonGDS BenSurgisonGDS force-pushed the plugin-validator-should-support-nunjucks-functions branch from 20728dd to e79b56f Compare November 17, 2023 09:31
Copy link
Contributor

@colinrotherham colinrotherham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, seems to run the checkPathExists() validation still

Would be great to maintain a JSDoc definition so we don't lose track of fields like this again in future? There's one I made already over in 39a3b41 which can help catch issues too

Otherwise the schema comment in plugins.js is very out of date:

* A schema for an example manifest file follows:
*
* // govuk-prototype-kit.config.json
* {
* "assets": path | path[],
* "importNunjucksMacrosInto": path | path[],
* "nunjucksPaths": path | path[],
* "nunjucksFilters": path | path[],
* "sass": path | path[],
* "scripts": path | path[],
* "stylesheets": path | path[],
* "templates": {
* "name": string,
* "path": path,
* "type": string
* }[]
* }

@BenSurgisonGDS
Copy link
Contributor Author

Looks good, seems to run the checkPathExists() validation still

Would be great to maintain a JSDoc definition so we don't lose track of fields like this again in future? There's one I made already over in 39a3b41 which can help catch issues too

Otherwise the schema comment in plugins.js is very out of date:

* A schema for an example manifest file follows:
*
* // govuk-prototype-kit.config.json
* {
* "assets": path | path[],
* "importNunjucksMacrosInto": path | path[],
* "nunjucksPaths": path | path[],
* "nunjucksFilters": path | path[],
* "sass": path | path[],
* "scripts": path | path[],
* "stylesheets": path | path[],
* "templates": {
* "name": string,
* "path": path,
* "type": string
* }[]
* }

Thank you for spotting this. I'll update this in a separate PR

@BenSurgisonGDS BenSurgisonGDS merged commit 71623da into main Nov 17, 2023
30 checks passed
@BenSurgisonGDS BenSurgisonGDS deleted the plugin-validator-should-support-nunjucks-functions branch November 17, 2023 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants